docs: add dependency management guidelines to CONTRIBUTING.md #1941
Conversation
Fixes prometheus#1913 Adds comprehensive guidance on managing dependencies in client_golang, based on learnings from the Slack discussion and recent dependency-related PRs. Covers the four key topics requested: 1. Vendoring vs. direct dependencies: - Prefer direct dependencies with Go modules - Vendoring only as last resort for critical cases - Go modules provide reproducible builds via go.sum 2. Using unstable dependencies: - Avoid pre-1.0, alpha/beta, and commit-based versions in production - Exceptions require strong justification and maintainer approval - Includes practical examples of good vs. bad dependency versions 3. Indirect dependencies: - KEY INSIGHT: Indirect deps don't always propagate to users! - Go modules only propagate deps that are actually used in import chain - Example: common's kingpin dependency doesn't leak to client_golang users - Practical commands: 'go mod why' and 'go mod graph' for analysis 4. Dependencies in different contexts: - Production code: Strictest (affects all users, requires approval) - Test code: Flexible (test deps don't leak to importers) - Examples: Most flexible (but imports CAN cause dep leakage) - Real example: Removing example_test.go imports cleaned up many deps The guidelines emphasize that client_golang is part of the Kubernetes dependency chain, making our dependency decisions impact thousands of projects. Includes the context that adding dependencies creates significant work for Kubernetes maintainers. Also covers: - Why we're strict (Kubernetes factor, transitive dep problem) - Dependency evaluation criteria - Update procedures - Red flags for rejecting dependencies - Practical examples and real cases from our history Signed-off-by: Sibasis Padhi <sibasis.padhi@gmail.com>
sibasispadhi
requested review from
ArthurSens,
bwplotka,
kakkoyun and
vesari
as code owners
|
@bwplotka tried to add the documentation as per slack discussion.Kindly take a look. |
bwplotka
left a comment
bwplotka
left a comment
There was a problem hiding this comment.
Thanks! Some comments to make it more readable.
Reviewed during our maintainers sync.
NOTE: It feels this was AI generated, which is fine as long as you double checked this closely. Next time please disclose the use of AI and the level of confidence you have in this change. We will likely add this to our CoC or policy one day clearly. (:
| - But `client_golang` users don't get `kingpin` as an indirect dependency because `client_golang` | ||
| doesn't import the parts of `common` that use `kingpin` | ||
|
|
||
| **Testing this:** |
There was a problem hiding this comment.
This is great, but either we add more specific info or even script for this or we remove this. It's a bit fuzzy e.g.
# See why a package is in our dependencies - is this from client_golang repo? Or from repo that imports client_golang?
Test if a dependency propagates by creating a test module that imports client_golang
and checking if the dependency appears in its go.mod
Kind of out of place? Can we give more specific output?
It feels a script would help?
There was a problem hiding this comment.
Updated inline. Let me know if you meant to be in a separate script ?
Address feedback from @bwplotka to add proper punctuation. Signed-off-by: Sibasis Padhi <sibasis.padhi@gmail.com>
The second bullet point was redundant with the first - both explained that Go modules only propagate dependencies that are actually used. Addresses feedback from @bwplotka. Signed-off-by: Sibasis Padhi <sibasis.padhi@gmail.com>
- Add missing trailing periods for consistency - Remove redundant bullet point about dependency propagation - Add permanent links and specific package examples (kingpin/promslog/flag) - Improve 'Testing this' section with concrete, actionable examples - Remove confusing 'exception' section about example files - Remove verbose sections (Unstable Dependencies, Update Process, Red Flags) - Remove AI-watery language and blog-post style sections - Reduce file from ~265 to 138 lines (~50% reduction) Addresses feedback from prometheus#1938 Signed-off-by: Sibasis Padhi <sibasis.padhi@gmail.com>
sibasispadhi
force-pushed
the
docs/add-dependency-management-guide
branch
from
a24d307 to
a9bc630
Compare
|
Hi @bwplotka — thank you for the detailed review. This was my first Go contribution, so I started with a documentation update. I did use GitHub Copilot for the changes, as AI-assisted workflows are becoming a common part of modern development. I was not aware that explicit disclosure was required. Regarding the AI-generated language in the documentation, that was my oversight. I did a high-level review but didn’t sufficiently refine or de-duplicate the content. I will be more careful about this in future contributions. On your point about potentially adding this to the CoC or policy in the future; it’s an interesting area. As AI-assisted tools become more common, which would generate more comments and documentation and it will be an important challenge to address. Thanks again for the feedback. |
2 participants
Fixes #1913
Adds comprehensive guidance on managing dependencies in client_golang, based on learnings from the Slack discussion and recent dependency-related PRs. Covers the four key topics requested:
Vendoring vs. direct dependencies:
Using unstable dependencies:
Indirect dependencies:
Dependencies in different contexts:
The guidelines emphasize that client_golang is part of the Kubernetes dependency chain, making our dependency decisions impact thousands of projects. Includes the context that adding dependencies creates significant work for Kubernetes maintainers.
Also covers: